Skip to content

Fix errors in the global initialization checker when compiling bootstrapped dotty #23429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

EnzeXing
Copy link
Contributor

@EnzeXing EnzeXing commented Jun 26, 2025

With this fix, the global initialization checker can be run when compiling bootstrapped dotty and reported warnings in the dotty source code

[test_scala2_library_tasty]

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

targetType.asInstanceOf[ExprType].resType
else
targetType.asInstanceOf[MethodType].resType
val returnType = targetType.finalResultType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simplification 👍

@@ -1326,7 +1318,8 @@ class Objects(using Context @constructorOnly):
report.warning("[Internal error] top-level class should have `Package` as outer, class = " + klass.show + ", outer = " + outer.show + ", " + Trace.show, Trace.position)
(Bottom, Env.NoEnv)
else
val outerCls = klass.owner.enclosingClass.asClass
// enclosingClass is specially handled for java static terms, so use `lexicallyEnclosingClass` here
val outerCls = klass.owner.lexicallyEnclosingClass.asClass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch and comment 👍

@olhotak olhotak assigned olhotak and unassigned noti0na1 Jun 27, 2025
Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM also except for a comment about why Flags.Param is an exception.

@EnzeXing EnzeXing force-pushed the fix-errors-when-testing-global-init-checker branch from 3c254fd to 83b6cf2 Compare July 2, 2025 03:36
@EnzeXing EnzeXing force-pushed the fix-errors-when-testing-global-init-checker branch from 83b6cf2 to ec0d1f9 Compare July 2, 2025 15:58
@@ -1054,6 +1046,8 @@ class Objects(using Context @constructorOnly):
else if target.equals(defn.Predef_classOf) then
// Predef.classOf is a stub method in tasty and is replaced in backend
UnknownValue
else if target.is(Flags.JavaDefined) then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the modification here to skip java-defined methods. It is worth noting that java-defined methods has source, but its defTree is incomplete, so we should not evaluate them anyway

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what way the defTree is incomplete. The definition of hasSource is:

    def hasSource(using Context): Boolean = !sym.defTree.isEmpty

But anyway, perhaps hasSource should check for Java-defined and return false for all Java-defined symbols. @liufengyun what do you think?

Copy link
Contributor Author

@EnzeXing EnzeXing Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the defTree of the runtimeVersionMajor method defined in JDK9Reflectors.java:

DefDef(runtimeVersionMajor,List(List(ValDef(version,Ident(Object),EmptyTree))),Ident(Integer),Select(Select(Select(Ident(_root_),scala),Predef),???))

The defTree is not empty, but contains an ??? part.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good idea to change hasSource such that it return false for all Java-defined symbols.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good idea to change hasSource such that it return false for all Java-defined symbols.

Done

@EnzeXing EnzeXing force-pushed the fix-errors-when-testing-global-init-checker branch from ec0d1f9 to 13440a7 Compare July 2, 2025 17:01
@olhotak olhotak merged commit 4b97d25 into scala:main Jul 2, 2025
29 checks passed
@olhotak olhotak deleted the fix-errors-when-testing-global-init-checker branch July 2, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants